Skip to content

Serialize CardViews missing from host tracker#10644

Open
MostCromulent wants to merge 1 commit intoCard-Forge:masterfrom
MostCromulent:fix-jhoira-ephemeral-cardview
Open

Serialize CardViews missing from host tracker#10644
MostCromulent wants to merge 1 commit intoCard-Forge:masterfrom
MostCromulent:fix-jhoira-ephemeral-cardview

Conversation

@MostCromulent
Copy link
Copy Markdown
Contributor

Closes #10641

Bug

In MoJhoSto network play, a remote player activating Jhoira of the Ghitu Avatar's ability (choose 1 of 3 random instant/sorcery copies to cast) never sees the choice dialog — the ability fizzles. PlayEffect creates the choice copies into ZoneType.None without calling Zone.add, so their CardViews never reach the host's tracker; when getChoices ships them as IdRefs, the client resolves them to null.

Fix

Skip IdRef replacement for CardViews that are absent from the host tracker. They serialize natively (full Java object) and the receiver registers them on arrival.

Two changes:

  • TrackableSerializer.replace(): in non-event mode, only emit IdRef when tracker.getObj(...) finds the entry. Otherwise return the object as-is so Java serialization writes the full CardView inline. PlayerView still uses IdRef unconditionally (always tracker-resident). Wrapped events keep the existing EventCardRef + preserveSnapshot logic (see "Why only non-event mode" below).
  • GameClientHandler.updateTrackers: while walking incoming protocol message args, also tracker.putObj(...) any TrackableObject not already in the id lookup. This registers inline-serialized CardViews so subsequent IdRef references targeting the same id (including the client's reply path) resolve correctly.

Why only non-event mode

Wrapped events use a separate EventCardRef mechanism that already carries name + image-key snapshots inline, so ephemeral references in events display correctly via the preserveSnapshot=false fallback. Events also need that snapshot mechanism for a different problem — stale CardView refs after zone changes — which would break under inline-serialization-plus-tracker-register: a stale snapshot from an event would overwrite the newer authoritative entry in the tracker, and subsequent IdRef resolutions for that id would return the stale view.

In practice, ephemeral CardViews are unlikely to be referenced in events anyway: the window where Jhoira's choice copies are tracker-miss is Card.fromPaperCard(...) → choice dialog → cast resolves, and during that window the cards aren't on the stack and aren't in any tracked zone.

Alternative considered

A different approach was to give updateGameView an overload accepting "extra" TrackableObjects, so callers could inject ephemeral views into the next DeltaPacket's newObjects. The client tracker would then be populated via the normal delta-sync new-object path, and a bare IdRef in the subsequent protocol method would resolve cleanly.

It was rejected because the implementation surface is larger and one piece of it is a real refactor of delta-sync timing.

The flow under the overload would be: build a delta carrying the ephemerals, send applyDelta, immediately send the protocol method that uses them. The two messages have to ship back-to-back with no game-thread work between them — the ephemerals only exist to support that one protocol call, so anything that lets them scatter into normal batching would defeat the point.

But with no game-thread work between the two sends, the client receives them back-to-back too — the IO thread decodes both before the EDT can apply the first. We're not aware of this race manifesting today because regular flows leave milliseconds of game-thread work between creating a card and prompting on it, giving the EDT time to catch up. The overload removes that buffer, so it also requires splitting applyDelta step 1 (create new objects, tracker.putObj) into beforeCall on the IO thread to keep ordering correct.

That split would create a window where new CardViews live in the tracker without their properties — every path that reads tracker entries between IO-thread beforeCall and EDT-side property application (afterDeltaApplied hooks, event dispatch ordering, reconnection state-reset, the in-place CardView-replacement-on-zone-change semantics in createObjectOnly) would need careful re-verification, with real risk of subtle regressions for a one-issue fix.

The inline-serialization approach sidesteps ordering entirely: the ephemeral CardView arrives as part of the protocol message itself, fully constructed before the message is dispatched. No new delta-sync code paths, and no applyDelta semantics shift.

Testing completed

  • mvn -pl forge-gui -am install -DskipTests clean
  • NetworkPlayIntegrationTest#runQuickDeltaSyncTest (10-game batch): 10/10 success, 0 errors, 0 checksum mismatches
  • Manual two-instance MoJhoSto with Fix lobby start crash for auto-generated deck variants #10643 stacked locally to allow auto-generated-deck variants to start: remote player activates Jhoira, choice dialog appears, selection casts and resolves

🤖 Generated with Claude Code

Skip IdRef replacement for CardViews absent from the host tracker so
ephemerals (e.g. Jhoira choice copies that never enter a tracked zone)
serialize natively. The receiver registers them in its tracker on
arrival so subsequent IdRef references resolve correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@MostCromulent MostCromulent requested a review from tool4ever May 9, 2026 22:16
@MostCromulent MostCromulent added Netplay BUG Something isn't working labels May 9, 2026
if (obj instanceof TrackableObject trackableObject) {
if (trackableObject.getTracker() == null) {
trackableObject.setTracker(this.tracker);
registerInTracker(trackableObject);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really hope we can avoid this part - it would essentially mean losing the optimization of #10343 on the client side for CardView...

can't we use this reasoning instead:

  • as long as hosts sends full objects we know they're ephemeral
  • if for some reason they end in up in the real game they would start to get tracked and reach client via normal delta path first anyway

with that client can instead perform an identical tracker check if he needs to respond to host with CardView (or IdRef is enough) 💡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BUG Something isn't working Netplay

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Network play: Remote player not presented with Jhoira choice dialog in MoJhoSto variant

2 participants